Add KeePass header check for testing remote download (#10910) #10915
Conversation
t-h-e
force-pushed
the
fix/10910-header-check
branch
from
6baf5c5
to
e209465
Compare
t-h-e
force-pushed
the
fix/10910-header-check
branch
from
e209465
to
695b550
Compare
| // Read public headers | ||
| QString error; | ||
| QScopedPointer<Database> db(new Database()); | ||
| return db->open(filePath, nullptr, &error); |
There was a problem hiding this comment.
You could inline this since you have only a single call for it and use the error string.
There was a problem hiding this comment.
I wasn't sure if I should use the error string, as it can be quite long and may not fit in the message box. I added the error anyway, as it is better to have it rather than leave the user in the dark.
I did not inline the method though, as I prefer having a named method, rather than calling Database::open with a nullptr as second parameter, which implies reading the headers.
There was a problem hiding this comment.
Since we are repeating this pattern we should introduce a Database::readHeaders function that takes care of the particulars.
| } | ||
|
|
||
| if (!hasValidPublicHeaders(result.filePath)) { | ||
| m_ui->messageWidget->showMessage(tr("Download finished, but file failed KeePass header check. File is not a " |
There was a problem hiding this comment.
You could use the actual error from database open here. "KeePass header check" is quite a technical error message as well.
There was a problem hiding this comment.
I have adapted the error message
t-h-e
force-pushed
the
fix/10910-header-check
branch
from
0378725
to
7753774
Compare
|
LGTM, I haven't yet tested this but the code look fine |
Verify downloaded file.
Context: #10910
Screenshots
Testing strategy
Test a command that downloads a random file that is not a KeePass database.
Type of change